Conversation
libbson 2.x's mlib/time_point.h needs POSIX feature-test macros defined directly rather than detected, and bson_as_json() was removed in favor of bson_as_json_with_opts().
- vendor.sh now diffs and reverts bsonjs/common/common-config.h the same way it already does for bsonjs/bson/config.h, instead of silently overwriting the hand-checked-in copy. - MANIFEST.in now includes *.th files so the new mlib/vec.th template ships in the sdist. - bsonjs.c uses bson_as_legacy_extended_json() instead of manually building bson_json_opts_t, matching the RELAXED/CANONICAL calls already in this function.
Uses drivers-github-tools' reusable codeql action, scanning our own
C code (bsonjs.c/bsonjs.h), the Python build/test glue, and the
workflow files themselves. Excludes bsonjs/{bson,jsonsl,common},
which are vendored libbson/jsonsl sources synced by vendor.sh rather
than code we maintain.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed portability/correctness issues in newly introduced/modified macros (clang deprecation pragma pop/push mismatch and overly broad BSON_HAVE_ALIGNED_ALLOC enabling) that can break builds or silently alter diagnostic behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the project’s bundled (vendored) libbson sources to 2.5.0 and adjusts the build, packaging, and wrapper code so the Python extension continues to compile and expose the same BSON↔Extended JSON functionality.
Changes:
- Bump vendored libbson from 1.27.2 to 2.5.0 and update vendoring script paths for the 2.x source layout.
- Update build configuration (feature-test macros) and packaging to accommodate new libbson/common/mlib headers and templates.
- Update the Python wrapper’s JSON conversion call and refresh documentation/changelog to reflect the new bundled version and benchmark results.
File summaries
| File | Description |
|---|---|
| vendor.sh | Update vendoring workflow for libbson 2.x layout and renamed generated headers. |
| setup.py | Add POSIX feature-test macros required by libbson 2.x clock/time code; refactor define_macros handling. |
| README.rst | Update libbson version link and benchmark claims/numbers. |
| MANIFEST.in | Include *.th template headers needed by mlib. |
| CHANGELOG.rst | Add 0.8.0 entry describing libbson bump and links to upstream changelog. |
| bsonjs/jsonsl/jsonsl.h | Update jsonsl header to include new libbson/common/mlib headers and adjust API/export details. |
| bsonjs/common/mlib/timer.h | Add vendored mlib timer utilities used by libbson 2.x. |
| bsonjs/common/mlib/str_vec.h | Add vendored mlib string-vector wrapper using vec.th. |
| bsonjs/common/mlib/platform.h | Add vendored mlib platform header for OS-specific includes/defines. |
| bsonjs/common/mlib/loop.h | Add vendored mlib loop/foreach macros. |
| bsonjs/common/mlib/intutil.h | Add vendored mlib integer utility macros/helpers. |
| bsonjs/common/mlib/cmp.h | Add vendored mlib safe integer comparison/range utilities. |
| bsonjs/common/common-utf8-private.h | Add vendored common UTF-8 helpers extracted from older bson-utf8 logic. |
| bsonjs/common/common-thread.c | Update vendored thread helpers to new header naming/includes and optional-param macro usage. |
| bsonjs/common/common-thread-private.h | Update vendored thread private header to new include structure and macro naming. |
| bsonjs/common/common-prelude.h | Update vendored prelude naming macros formatting and copyright. |
| bsonjs/common/common-oid.c | Add vendored OID helper implementation (zero OID helpers). |
| bsonjs/common/common-oid-private.h | Add vendored OID helper declarations. |
| bsonjs/common/common-md5-private.h | Update vendored md5 private header and move md5 struct typedef here. |
| bsonjs/common/common-macros-private.h | Expand vendored private macros for diagnostics/warnings control. |
| bsonjs/common/common-bits-private.h | Add vendored bit-manipulation helper(s). |
| bsonjs/common/common-b64-private.h | Update vendored base64 private header naming and macro formatting. |
| bsonjs/common/common-atomic.c | Add vendored atomic emulation/helpers replacing prior bson atomic impl. |
| bsonjs/bsonjs.c | Switch legacy-mode JSON serialization to libbson 2.x legacy JSON API. |
| bsonjs/bson/version.h | Bump BSON version macros/strings to 2.5.0. |
| bsonjs/bson/validate-private.h | Add vendored validate private API/constants for new validation implementation. |
| bsonjs/bson/memory.h | Add vendored memory API header (replaces removed bson-memory.h). |
| bsonjs/bson/memory.c | Update vendored memory implementation (aligned alloc selection, array alloc helpers, vtable). |
| bsonjs/bson/error.h | Add vendored public error API header (replaces older bson-error header layout). |
| bsonjs/bson/error.c | Update vendored error implementation (category field, strerror portability paths). |
| bsonjs/bson/config.h | Update hand-maintained config header; add BSON_HAVE_ALIGNED_ALLOC knob. |
| bsonjs/bson/compat.h | Update vendored compat header includes/exports and platform defines. |
| bsonjs/bson/bson-writer.h | Update writer header includes and formatting; use new memory/bson_t headers. |
| bsonjs/bson/bson-writer.c | Update writer implementation to new bson_t private layout and flags. |
| bsonjs/bson/bson-version-functions.h | Update version-functions header formatting. |
| bsonjs/bson/bson-version-functions.c | Update version-functions implementation includes and formatting. |
| bsonjs/bson/bson-vector-private.h | Add vendored private helpers for BSON vector element header bytes. |
| bsonjs/bson/bson-value.h | Update bson-value header includes/formatting. |
| bsonjs/bson/bson-value.c | Update value copy/destroy implementation (range checks, allocations, includes). |
| bsonjs/bson/bson-utf8.h | Update utf8 header includes/formatting. |
| bsonjs/bson/bson-utf8.c | Refactor utf8 implementation to use common UTF-8 and common JSON escaping helpers. |
| bsonjs/bson/bson-timegm-private.h | Update timegm private header includes/formatting. |
| bsonjs/bson/bson-string.h | Simplify public string header to core string APIs; remove bson_string_t API from public header. |
| bsonjs/bson/bson-reader.h | Update reader header to use BSON_CALL function pointer typedefs and compat include. |
| bsonjs/bson/bson-private.h | Remove old private bson implementation header (replaced by bson_t-private.h). |
| bsonjs/bson/bson-prelude.h | Update vendored prelude header copyright. |
| bsonjs/bson/bson-oid.h | Update OID header includes/formatting and unsafe-buffer-usage warning macros. |
| bsonjs/bson/bson-oid.c | Update OID implementation (init logic, includes/formatting). |
| bsonjs/bson/bson-memory.h | Remove old bson-memory public header (replaced by bson/memory.h). |
| bsonjs/bson/bson-md5.h | Remove deprecated md5 public header. |
| bsonjs/bson/bson-md5.c | Remove deprecated md5 shim implementation. |
| bsonjs/bson/bson-keys.h | Update keys header includes/formatting. |
| bsonjs/bson/bson-keys.c | Update uint32-to-string implementation to assert/return size_t safely. |
| bsonjs/bson/bson-json.h | Update JSON API header includes and BSON_CALL typedefs/formatting. |
| bsonjs/bson/bson-json-private.h | Ensure JSON private header exports public JSON header. |
| bsonjs/bson/bson-iso8601.c | Update ISO8601 parse implementation includes/formatting; remove date format helper implementation. |
| bsonjs/bson/bson-iso8601-private.h | Remove ISO8601 date format helper declaration; update includes. |
| bsonjs/bson/bson-error-private.h | Update error private header to export bson/error.h and add category setter helper. |
| bsonjs/bson/bson-endian.h | Update endian header; add float endian-swap support and modernize include structure. |
| bsonjs/bson/bson-decimal128.h | Update decimal128 header includes/formatting. |
| bsonjs/bson/bson-context.h | Update context header includes/formatting. |
| bsonjs/bson/bson-context-private.h | Update context-private header includes; remove deprecated seq64 helper declaration. |
| bsonjs/bson/bson-cmp.h | Remove old bson-cmp helpers (replaced by mlib cmp utilities). |
| bsonjs/bson/bson-clock.h | Update clock header includes/formatting. |
| bsonjs/bson/bson-clock.c | Update monotonic time implementation to use mlib time_point/duration utilities. |
| bsonjs/bson/bson-bcon.h | Add vendored BCON declarations header in new location/name. |
| bsonjs/bson/bson-atomic.c | Remove old bson atomic implementation (replaced by common-atomic.c). |
| bsonjs/bson/bson_t.h | Add new public bson_t definition header. |
| bsonjs/bson/bson_t-private.h | Add new private bson_t implementation details header (replaces bson-private.h). |
| bsonjs/bson/bcon.h | Remove old bcon.h location (replaced by bson-bcon.h). |
Review details
Suppressed comments (1)
bsonjs/common/common-macros-private.h:78
- In the clang branch, END_IGNORE_DEPRECATIONS expands to a diagnostic push instead of a pop, so the push/pop pair is unbalanced and could leave warnings suppressed (or cause pragma-stack warnings) if these macros are used.
- Files reviewed: 104/105 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The workflow's paths-ignore config didn't keep two alerts in vendored libbson files (bsonjs/bson/bson-decimal128.c, bson-timegm.c) out of the c-cpp analysis, despite those paths being listed there and confirmed present in CodeQL's own augmented config. .codeqlignore is applied before extraction and is the mechanism GitHub documents as reliable for compiled languages.
…g them
paths-ignore only filters results for interpreted languages or
build-mode: none — it doesn't apply when build-mode is manual/autobuild,
per GitHub's own docs, so it never excluded bsonjs/bson/*.c from the
c-cpp analysis despite listing them. .codeqlignore is likewise a no-op
here since it's only read by GitHub's Default Setup, which this repo
doesn't use.
CodeQL only extracts files the compiler is actually invoked on, so the
c-cpp job now compiles just bsonjs.c directly instead of running the
full `pip install -e .`, which also built every vendored .c file under
bsonjs/{bson,jsonsl,common}.
There was a problem hiding this comment.
🟡 Changes recommended
The new CodeQL workflow’s permissions are likely insufficient for repository checkout/analysis, and there is a confirmed Clang diagnostic macro bug (END_IGNORE_DEPRECATIONS) that should be fixed to avoid unbalanced diagnostic state.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
bsonjs/common/common-macros-private.h:79
- For the Clang branch, END_IGNORE_DEPRECATIONS expands to a diagnostic push instead of a pop, which will unbalance the diagnostic stack and can affect subsequent warning state in any TU that uses these macros.
- Files reviewed: 104/105 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new CodeQL workflow YAML is invalid (matrix/strategy mis-indented) and there is a confirmed uninitialized-memory copy in bsonjs/bson/error.c that can leak stack data into bson_error_t.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
bsonjs/common/common-macros-private.h:79
- For Clang builds,
END_IGNORE_DEPRECATIONSis defined asMC_PRAGMA_DIAGNOSTIC_PUSH, which leaves the diagnostic state unbalanced and defeats the purpose of the push/pop pair. It should pop.
bsonjs/common/mlib/loop.h:92 - Spelling typo in comment: "iteraiton" should be "iteration".
- Files reviewed: 104/105 changed files
- Comments generated: 2
- Review effort level: Lite
fail-fast and matrix had ended up nested under permissions instead of strategy, so the workflow failed to parse.
There was a problem hiding this comment.
🔵 Needs a closer look
bsonjs/common/common-macros-private.h defines END_IGNORE_DEPRECATIONS as a diagnostic push under __clang__, which is an objective macro bug that can leave diagnostic state unbalanced and should be corrected before merging.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
bsonjs/common/common-macros-private.h:78
- In the clang branch, END_IGNORE_DEPRECATIONS is defined as MC_PRAGMA_DIAGNOSTIC_PUSH, which does not restore the prior diagnostic state and can leave warning state permanently altered (and unbalanced pushes) in translation units that use these macros. It should pop the diagnostic state instead.
- Files reviewed: 104/105 changed files
- Comments generated: 0 new
- Review effort level: Lite
bson-bcon.c (BCON), common-md5.c, and common-thread.c implement libbson features bsonjs never calls, and nothing else in the vendored tree references their symbols either. Verified via nm against the rest of the built extension and confirmed by import + the test suite. bson-vector.c looked like another candidate by the same criteria, but bson.c's array builder actually calls into it (bson_append_array_from_vector), so it stays.
vendor.sh now deletes bson-bcon.c, common-md5.c, and common-thread.c right after syncing them, so future vendor bumps keep them out automatically instead of needing setup.py to filter them out of the build every time. Their headers stay: bson.h includes bson-bcon.h, and common-thread-private.h is a real dependency of bson-context.c and common-b64.c. setup.py's sources glob goes back to unfiltered, now that the files it would have excluded no longer exist in the tree.
ccache is only used as a compiler launcher on GNU/Clang platforms; its MSVC launcher hangs CMake's compiler detection on Windows. Stop installing it there so the Windows jobs don't pay the choco install for an unused tool.
CI runners (especially Windows) can stall for 20+ minutes on the FetchContent download of the mongo-c-driver tarball with no output. Download with hard timeouts (TIMEOUT/INACTIVITY_TIMEOUT) and retries so a stall fails fast instead of hanging the build. Also cache the release tarball via a setup-mongo-c-driver action and feed it to CMake through BSONJS_MCD_TARBALL, so later runs skip the download entirely.
The BSONJS_MCD_TARBALL override uses a Windows path with backslashes, which broke the FetchContent subbuild's generated CMakeLists.txt (syntax error). Convert it with file(TO_CMAKE_PATH) and only skip the download when the normalized path actually exists.
Hardens the download so CI never silently falls back to the stalling codeload endpoint mid-build. setup-mongo-c-driver now fails the job (set -euo pipefail + test -s) if the release tarball can't be fetched within the retry/timeout budget, and is wired into the dist.yml wheel build too so the host builds (macOS/Windows) pre-fetch it. CMakeLists still prefers the pre-fetched archive; the timeout-bounded download stays as a fallback for dist.yml Linux containers and sdist/local builds, which can't run the action but don't stall on codeload.
GitHub does not stream the in-build CMake output until a job finishes, so a stall is invisible. Add [bsonjs] phase markers (configure begin, archive source + size, download, FetchContent_MakeAvailable begin/done) with an elapsed counter to CMakeLists, and log cache-vs-download in setup-mongo-c-driver. The next run's log will show exactly which phase consumes the time on the hosted runner.
The pip build-isolation env (fresh venv + tmp dir re-installing scikit-build-core/cmake/ninja each run) is a suspect for the slow Windows build. Pre-install the build deps and build with --no-build-isolation, so the wheel builds in the repository's own build/ dir. Verified on the Windows host: the cp311-abi3 wheel builds in ~18s. Also guard the ccache stats step off Windows, where ccache is no longer installed.
The ENABLE_MONGOC/ENABLE_SHARED/ENABLE_STATIC options were set AFTER FetchContent_MakeAvailable, so they were never honored and mongo-c-driver defaulted to building libmongoc (plus zlib, utf8proc, SASL, and the shared variants) — ~300 C files instead of the ~30 for libbson. That is why the GitHub-hosted Windows build stalls (a 10x MSVC workload) and why the Linux dist wheel fails with "MONGODB-AWS requires a crypto library". Move the options above the single MakeAvailable so the subproject configures libbson-only, and pin the same defines in pyproject (cmake.define) so every build path configures the same minimal build.
GitHub-hosted Windows real-time MSVC compilation is throttled by Defender, which is the classic cause of 10-50x slowdowns. Exclude the checkout from Defender scanning before the build. Combined with building only libbson, this targets the residual slow compile on hosted runners.
Diagnostic: print whether cl/gcc are on PATH and which C compiler CMake picks, so we can confirm the GitHub-hosted Windows runner is building with MSVC and not MinGW-w64 gcc (which is preinstalled alongside Visual Studio).
Diagnosis: scikit-build-core uses Ninja, which picks the compiler from PATH. On GitHub-hosted Windows `cl` is not on PATH and C:\\mingw64\\bin\\gcc is, so the build was compiling with MinGW gcc (GNU 12.2.0), not MSVC — a different toolchain that behaved unlike the box. Set up the MSVC dev environment (vcvars64 via vswhere) before pip wheel so cl.exe is on PATH and scikit-build-core builds a proper MSVC extension.
scikit-build-core's Ninja build picked MinGW gcc on Windows runners, producing a GNU wheel instead of an MSVC one. Meson with --vsenv forces the Visual Studio toolchain. The Meson build compiles libbson from the vendored mongo-c-driver source and tags the module cp311-abi3.
error.c needs _XOPEN_SOURCE >= 700 or _GNU_SOURCE for strerror_l, so use _GNU_SOURCE on non-Apple POSIX hosts. Windows extraction broke because Git-bash tar reads the colon in a drive-letter path as a remote host; extract with Python tarfile instead.
Meson 1.12 cannot parse the /showIncludes dependency prefix that the windows-latest runner's new MSVC 19.51 (VS 2026) emits, so dep-prefix detection aborts. windows-2022 provides VS 2022 (MSVC 17.x), which Meson handles.
windows-2022 has no ccache preinstalled, so the setup-ccache action's ccache --zero-stats step exits 127. ccache is a GNU/Clang launcher and is not used by the MSVC/Meson Windows build, so don't set it up there.
libbson's bson-context.c calls gethostname, which needs ws2_32.lib on Windows. A bare 'ws2_32' link arg made the MSVC linker look for ws2_32.obj and fail with LNK1181; find_library resolves it correctly.
The Meson migration replaced CMakeLists.txt, so build docs and the libbson bump helper now reference meson.build. Removes the leftover BSONJS_MCD_TARBALL env var that Meson no longer reads.
Add scripts/fetch_mongo_c_driver.py, shared by the meson build, the CI setup action, and CodeQL. It reads the version and SHA256 pin from meson.build, so bump_libbson.py updates both and no caller hardcodes a version. meson.build fetches the pinned release when .mongo-c-driver/ is absent, so an sdist can build itself. Correct the source-build wording, require meson 1.3 for limited_api, and fix the stale CMake references.
meson-python cannot find the 32-bit Python headers in the 32-bit cibuildwheel environment, so the win32 build fails. Restrict the Windows build to AMD64 and note the dropped architecture in the changelog.
Meson cannot parse the /showIncludes dependency prefix that the windows-latest runner's new MSVC 19.51 (VS 2026) emits, so dependency detection aborts. windows-2022 provides VS 2022 (MSVC 17.x), which Meson handles. The test workflow already pins windows-2022 for the same reason.
windows-2022 has no ccache preinstalled, so the setup-ccache action's ccache --zero-stats step exits 127. ccache is a GNU/Clang launcher and is not used by the MSVC/Meson Windows build, so don't set it up there. The test workflow already guards this.
musl does not alias _GNU_SOURCE to _DEFAULT_SOURCE, so libbson's mlib time_point.h fails its POSIX clock check and the musllinux aarch64 build aborts. glibc aliases the two, which is why manylinux passes. Also run the dist wheel matrix with fail-fast disabled so one failing platform does not cancel the others.
libbson's error.c selects strerror_l only when _XOPEN_SOURCE >= 700. musl does not derive that from _GNU_SOURCE, so the musllinux build falls back to glibc-style strerror_r and fails to compile. glibc derives it, which is why manylinux passes.
Meson's --vsenv activates only the x64 toolset, and its Python dependency
check rejects a 32-bit Python against an x64 compiler ("Need python for
x86_64, but found x86"), which is why the 64-bit-only workaround landed.
The dist workflow now runs a second cibuildwheel pass for x86 with the
x86 toolset exported first, so Meson skips its own activation and
detects the host as x86.
Name the exact release each action SHA resolves to (v7.0.1 and v8.0.1), and give dist.yml and release-python.yml minimal top-level permissions so jobs no longer inherit the default token scope.
Summary
Bump the bundled libbson from 1.27.2 to 2.5.3 and build it from the pinned mongo-c-driver release instead of vendoring its C sources. The build backend moves to Meson.
Motivation
Vendoring libbson meant re-syncing and hand-patching its sources on every bump. Consuming the released mongo-c-driver sources directly removes that maintenance.
Changes
scripts/fetch_mongo_c_driver.py, which downloads the pinned release, verifies its SHA256, and extracts it..mongo-c-driver/is absent, so an sdist can build itself.scripts/bump-libbson.sh,scripts/bump_libbson.py).Testing
python -m pytest test/— 23 passed..mongo-c-driver, and built a wheel from it; the build downloaded, verified, and compiled libbson.Breaking changes